SBOM tab: add match condition filtering#461
Conversation
d9c6e7d to
73f6ca1
Compare
ValentinBoudevinSFL
left a comment
There was a problem hiding this comment.
The feature is well covered, but it currently introduces a high-cost server evaluation path, accepts undocumented ordering literals, and can display stale asynchronous results. These issues should be addressed before merge.
| if ( | ||
| not isinstance(item, dict) | ||
| or not isinstance(item.get("id"), str) | ||
| or not isinstance(item.get("data"), dict) |
There was a problem hiding this comment.
The condition is parsed again for every item because parser.evaluate(condition, ...) receives the source string inside the loop. At the permitted 50,000 items and 1,000-character conditions, one request can trigger tens of millions of parsing operations and become a CPU DoS vector. Parse and validate the expression once, then evaluate the parsed representation for each item's data.
| if condition[1] == ">=": | ||
| return self._eval_internal(condition[0]) >= self._eval_internal(condition[2]) # type: ignore[operator] | ||
| return self._compare_ordered( | ||
| condition[1], |
There was a problem hiding this comment.
Ordering comparisons pass unknown_as_literal=True, so an unknown RHS is accepted whenever the left value is also orderable as a string (for example, id > TYPO performs a lexical comparison). This contradicts the documented rule that literals are only allowed with ==/!= and can hide typos. Evaluate ordered RHS expressions without unknown_as_literal.
| setMatchConditionError(''); | ||
| try { | ||
| setMatchingVulnerabilityIds(await Vulnerabilities.matchCondition(condition, vulnerabilities)); | ||
| } catch (error) { |
There was a problem hiding this comment.
An in-flight evaluation can restore stale results after vulnerabilities changes: the effect clears the IDs, but the old request later resolves and calls setMatchingVulnerabilityIds. Multiple requests can also complete out of order because Enter remains active while the Apply button is disabled. Track a request generation or use an AbortController, and ignore results when the vulnerability snapshot or latest request has changed.
73f6ca1 to
1f0a49e
Compare
ValentinBoudevinSFL
left a comment
There was a problem hiding this comment.
The filtering flow is mostly coherent, but drill-down filters can become stale after vulnerability data refreshes.
| @@ -228,7 +229,8 @@ function Explorer({ darkMode, setDarkMode }: Readonly<Props>) { | |||
| setTab('vulnerabilities'); | |||
| } | |||
There was a problem hiding this comment.
The matching-ID snapshot remains stored after drilling down, while TablePackages is unmounted. A subsequent vulnerability refresh therefore leaves TableVulnerabilities filtering fresh data with stale IDs. Clear filterVulnerabilityIds when the vulnerability snapshot or scope changes.
1f0a49e to
21b6715
Compare
ValentinBoudevinSFL
left a comment
There was a problem hiding this comment.
The feature looks good to me (cvss works now).
But the frontend need to receive some clarity:
1- Search Syntax is only applicable to the search bar, should stay next to it
2- Add another information mark next to match condition to provide some explanations
3- We need to find a way to make obvious the fact the Apply button is only for Match Condition and not the search bar
450a20f to
2898bc5
Compare
ValentinBoudevinSFL
left a comment
There was a problem hiding this comment.
Frontend looks good
Match condition field looks good
But, it doesn't respect dod with "Dod: We have an efficiant way to get all remaining assessments to do based on a generic condition"
After this match condition entered I should get a new column, or something else, to get the quantity of Vulnerabilities per package which match this condition.
Right now, it only get rid of the packages which doesn't have a vulnerability which respect it
2898bc5 to
9308936
Compare
|
@ValentinBoudevinSFL I added an small design change to the existing Vulnerabilities to indicate a match condition count, details shown when you hover.
|
ValentinBoudevin
left a comment
There was a problem hiding this comment.
LGTM can you just remove the column "remaining pending vulnerabilities"
9308936 to
c368fbb
Compare
2b49bd9 to
e241d10
Compare
e241d10 to
0e521d4
Compare
67ccc5c
into
savoirfairelinux:staging


Fixes # by...
Changes proposed in this pull request:
SBOM tab: add match condition filtering
Status
How to verify this change
cvss >= 9in Match condition then click Apply.cvss >= 9are displayed.The PR supports the following conditions, the same as those supported in the reports and CI mode:
A zero-result condition is valid. For example:
This also supports nested Condition
When you input invalid Conditions you will get an error:
Finally, click Reset Filters and confirm the condition and filtered results are cleared.
Pull Request Checklist
Please review and check all that apply before submitting your PR: